-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(output): Remove Refreshing state... from output #1352
fix(output): Remove Refreshing state... from output #1352
Conversation
Since Terraform 0.14.0 there are no separator between refreshing plan and the plan.
a6e8f37
to
7a8a236
Compare
I update the PR to check if Terraform is in version >= 0.14.0. |
@lkysow Do you have time to check this? |
this would be greatly appreciated - we upgraded to TF 0.14.x to get the concise diffs but now we have a wall of Terraform refresh text output 🙏🏼 |
@chenrui333 Hi! would it be possible to get your eyes on it? we'd love to help in any way, and appreciate any time you might spend on this. Thank you! |
@nishkrishnan Could you check this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good, I see that you've been submitting a bunch of PRs around 0.14.0 improvements and thanks for your contribution. But in these cases where there are lot of if and elses being done to preserve backwards compatibility I'd suggest something along the lines of using composition to abstract the underlying implementation and wrapping them up in a nicely packaged interface so we can proxy to implementations based on a condition. Will make things more readable and easy to add changes for future versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 lgtm
Thanks @chenrui333 and @nishkrishnan, definitely appreciate it! 🙇 |
This reverts commit 7301feb.
you're gonna need to update test fixtures and then I can merge it back in |
What? |
The tests failed on master so I'm reverting the merge. |
I'm sorry, that the line 101, my editor remove the last space automatically. I fix that. |
I'm reopen a new PR? |
Fixes #1306
Since Terraform 0.14.0 there are no separator between refreshing plan and the plan. This strategy is to find the last line with "Refreshing state..." and remove it with all of the above.
I remove the sentence "Refreshing Terraform state in-memory prior to plan..." from tests because Terraform don't show that anymore.